feat(react-card): add `selectable` prop #23349

theerebuss · 2022-06-01T13:14:54Z

New Behavior

Added the selectable property which will control:

If we show a checkbox on the Card
Apply an onClick and an onKeyDown(Enter key only) event on the card element and an onChange event to the checkbox. These will all flip the selected state's value and then call the onCardSelect prop method
Apply Fluent styles for when card is selected

Related Issue(s)

#19336

fabricteam · 2022-06-01T13:22:43Z

📊 Bundle size report

Package & Exports	Baseline (minified/GZIP)	PR	Change
`react-card` Card - All	`66.137 kB` `18.69 kB`	`92.101 kB` `24.998 kB`	`25.964 kB` `6.308 kB`
`react-card` Card	`61.293 kB` `17.567 kB`	`87.498 kB` `23.985 kB`	`26.205 kB` `6.418 kB`

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-card` CardFooter	`7.737 kB` `3.264 kB`
`react-card` CardHeader	`9.302 kB` `3.779 kB`
`react-card` CardPreview	`7.838 kB` `3.316 kB`
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`185.285 kB` `51.364 kB`
`react-components` react-components: FluentProvider & webLightTheme	`33.988 kB` `11.108 kB`

_{🤖 This report was generated against a827840680beff7297d0fc27425ea20426a00de3}

size-auditor · 2022-06-01T13:28:45Z

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: a827840680beff7297d0fc27425ea20426a00de3 (build)

fabricteam · 2022-06-01T13:29:02Z

Perf Analysis (`@fluentui/react-components`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	1363	1382	5000
Button	mount	1021	1045	5000
FluentProvider	mount	2285	2235	5000
FluentProviderWithTheme	mount	742	753	10
FluentProviderWithTheme	virtual-rerender	714	730	10
FluentProviderWithTheme	virtual-rerender-with-unmount	786	755	10
MakeStyles	mount	2017	2052	50000

codesandbox-ci · 2022-06-02T11:01:24Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9b0e88e:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

theerebuss · 2022-06-06T06:45:37Z

packages/react-components/react-card/e2e/Card.e2e.tsx

        </div>
        <CardFooter>
-          <Button id="open-button" onClick={alert} appearance="primary">
+          <Button id="open-button" onClick={() => console.log('open-button clicked')} appearance="primary">


Avoid usage of alert.

wondering what is the best practice here ?

I found that alert doesn't work with Cypress (on WSL at least) so I went with something more reliable. Output isn't really used for anything, so maybe we remove it?

theerebuss · 2022-06-06T06:54:09Z

packages/react-components/react-card/src/components/Card/useCardStyles.ts

-  interactiveNoOutline: {
-    ':hover::after': {
-      ...shorthands.borderColor(tokens.colorTransparentStrokeInteractive),
-    },
-    ':active::after': {
-      ...shorthands.borderColor(tokens.colorTransparentStrokeInteractive),
-    },
-  },


Merged into each separate appearance style to simplify assignment logic later on.

packages/react-components/react-card/src/components/Card/useCardStyles.ts

theerebuss · 2022-06-06T06:55:12Z

packages/react-components/react-card/src/components/Card/useCardStyles.ts

-  const interactive =
-    state.root.onClick ||
-    state.root.onMouseUp ||
-    state.root.onMouseDown ||
-    state.root.onPointerUp ||
-    state.root.onPointerDown ||
-    state.root.onTouchStart ||
-    state.root.onTouchEnd;


selectable prop now controls intractability.

Actually, this was an incorrect assumption. Card should still get interactive styles without selectable if it is "interactive". We'll need to double check what interactive means but for now we'll keep this logic.

packages/react-components/react-card/src/components/Card/useCardStyles.ts

Hotell · 2022-06-06T13:55:37Z

packages/react-components/react-card/e2e/Card.e2e.tsx

        </div>
        <CardFooter>
-          <Button id="open-button" onClick={alert} appearance="primary">
+          <Button id="open-button" onClick={() => console.log('open-button clicked')} appearance="primary">


wondering what is the best practice here ?

packages/react-components/react-card/e2e/Card.e2e.tsx

packages/react-components/react-card/src/components/Card/useCardStyles.ts

theerebuss · 2022-06-06T14:58:23Z

apps/vr-tests-react-components/src/stories/Card.stories.tsx

      </Card>
    ),
    {
-      includeRtl: true,


RTL not necessary, already tested in non-interactive test.

theerebuss · 2022-06-06T15:20:28Z

packages/react-components/react-card/src/components/Card/useCard.ts

+          onClick: onChangeHandler,
+          onKeyDown: (event: React.KeyboardEvent<HTMLDivElement>) => {
+            if (event.key === Enter || event.key === Space) {
+              event.preventDefault();


Not a fan at all of preventing browser defaults but, if we don't, it will otherwise collide with our interception of Spacebar usage. Open to more elegant solutions.

q: curious if we might have this funcionality somewhere else already so we can make it generic and reuse for consistency?

I checked the demos and run into interesting situation. is this supposed to work like following ?

Not a fan at all of preventing browser defaults but, if we don't, it will otherwise collide with our interception of Spacebar usage

what behaviour are you referring to, scrolling the page ?

Under the focus of a checkbox, as you mentioned, Spacebar already checks it. This will check the checkbox, but the event is then bubbled up to the Card handler and it gets checked again.
I'll need some time to analyze this and find out a better approach.

I'm also now thinking about how this will interact with the requested focus behaviors, given that Enter is supposed to "go into the card" and focus the inner elements. I guess we need to bring this up with design and clarify.

yeah, to me that behaviour is kinda confusing as both events will trigger (selectable and action click) which also ads some lag (freezes UI for second or so - might be Storybook related though). I'd expect to not mark it as selected if I click on action control

Hotell · 2022-06-07T07:50:44Z

packages/react-components/react-card/src/components/Card/useCard.ts

+          onClick: onChangeHandler,
+          onKeyDown: (event: React.KeyboardEvent<HTMLDivElement>) => {
+            if (event.key === Enter || event.key === Space) {
+              event.preventDefault();


q: curious if we might have this funcionality somewhere else already so we can make it generic and reuse for consistency?

Hotell · 2022-06-07T07:54:51Z

packages/react-components/react-card/src/components/Card/useCardStyles.ts

 export const useCardStyles_unstable = (state: CardState): CardState => {
  const styles = useStyles();

+  const hasInteraction =


wondering if this is what we want, enabling "magic" behaviour based on some props defined ? should we be rather explicit with our APIs ?

Do you mean having an interactive prop that would enable said styles? I'm afraid that if we want to guarantee interaction styles and we need to rely on the user knowing beforehand such a prop is required will only lead to misalignment between intended design behavior and the use case.
This would be the result:

<Card interactive onClick={() => {}}>{...}</Card> <Card interactive selectable selected={selected} onCardSelect={() => {}}>{...}</Card>

If the user misses this, they get no cursor: pointer / styles when clicking and when selected.

What I meant is that in current state, the API is not explicit (mainly from Types API surface perspective). Based on this logic I don't have to provide selectable only some of the event handlers you're checking for, which will turn that behaviour on.

Do you mean having an interactive prop that would enable said styles?

yes to have this explicit if possible

Hotell · 2022-06-07T08:12:14Z

packages/react-components/react-card/src/components/Card/useCard.ts

+          onClick: onChangeHandler,
+          onKeyDown: (event: React.KeyboardEvent<HTMLDivElement>) => {
+            if (event.key === Enter || event.key === Space) {
+              event.preventDefault();


I checked the demos and run into interesting situation. is this supposed to work like following ?

Hotell · 2022-06-07T08:13:10Z

packages/react-components/react-card/src/components/Card/useCard.ts

+          onClick: onChangeHandler,
+          onKeyDown: (event: React.KeyboardEvent<HTMLDivElement>) => {
+            if (event.key === Enter || event.key === Space) {
+              event.preventDefault();


Not a fan at all of preventing browser defaults but, if we don't, it will otherwise collide with our interception of Spacebar usage

what behaviour are you referring to, scrolling the page ?

Co-authored-by: Martin Hochel <hochelmartin@gmail.com>

msft-fluent-ui-bot assigned ling1726 Jun 1, 2022

theerebuss unassigned ling1726 Jun 1, 2022

github-actions bot added this to the June Project Cycle Q2 2022 milestone Jun 2, 2022

theerebuss commented Jun 6, 2022

View reviewed changes

theerebuss added Component: Card Fluent UI react-components (v9) labels Jun 6, 2022

theerebuss mentioned this pull request Jun 6, 2022

Card Convergence #19336

Closed

37 tasks

theerebuss marked this pull request as ready for review June 6, 2022 12:49

theerebuss requested a review from a team as a code owner June 6, 2022 12:49

Hotell reviewed Jun 6, 2022

View reviewed changes

theerebuss requested a review from a team as a code owner June 6, 2022 14:18

theerebuss commented Jun 6, 2022

View reviewed changes

Hotell reviewed Jun 7, 2022

View reviewed changes

theerebuss marked this pull request as draft June 7, 2022 11:46

theerebuss added 11 commits June 14, 2022 17:48

feat: add new props to API

fa5b38e

chore: add new props to the component hook

359182d

chore: improve styling approach

e7f05d0

chore: add slot render

e3b082e

chore: add selectable story

cd5a709

chore: apply selected styles when checked

88f6706

chore: add all appearances to story

20d4322

chore: move select slot rendering to avoid z-index usage

7b8a5d6

chore: add change file

92e713f

chore: update API files

f85bf7b

chore: trigger change only on Enter press

5851cc9

theerebuss and others added 16 commits June 14, 2022 17:54

chore: use @fluentui/keyboard-keys for consistency

584d425

chore: add missing dependency

bf00233

fix: make changes conformant

98c98ee

chore: add e2e tests

f8b84eb

chore: revert useCard back to .ts

21a194a

fix: use nullish operator for selection state initialization

44dace1

feat: use react-checkbox for the selection

3e63168

chore: add documentation

3324522

fix: correct interactive tests

27718c3

Update packages/react-components/react-card/e2e/Card.e2e.tsx

713b3d8

Co-authored-by: Martin Hochel <hochelmartin@gmail.com>

fix: reintroduce interactive behavior

dab2b6b

feat: add Spacebar selection

641369d

feat: add focusMode + selectable interaction

bb62575

chore: update API

8af1b84

chore: add e2e tests

8736b91

chore: add vr test for new behavior

9b0e88e

tudorpopams assigned theerebuss Jul 7, 2022

theerebuss added Fluent UI react-components (v9) Fluent UI react-components (v9) and removed Fluent UI react-components (v9) Fluent UI react-components (v9) labels Jul 13, 2022

marcosmoura self-assigned this Jul 21, 2022

marcosmoura removed their assignment Aug 23, 2022

theerebuss closed this by deleting the head repository Sep 23, 2022

feat(react-card): add selectable prop #23349

feat(react-card): add selectable prop #23349

Uh oh!

Conversation

theerebuss commented Jun 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New Behavior

Related Issue(s)

Uh oh!

fabricteam commented Jun 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

size-auditor bot commented Jun 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

fabricteam commented Jun 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codesandbox-ci bot commented Jun 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theerebuss Jun 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theerebuss Jun 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hotell Jun 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

feat(react-card): add `selectable` prop #23349

feat(react-card): add `selectable` prop #23349

theerebuss commented Jun 1, 2022 •

edited

Loading

fabricteam commented Jun 1, 2022 •

edited

Loading

size-auditor bot commented Jun 1, 2022 •

edited

Loading

fabricteam commented Jun 1, 2022 •

edited

Loading

codesandbox-ci bot commented Jun 2, 2022 •

edited

Loading

theerebuss Jun 6, 2022 •

edited

Loading

theerebuss Jun 7, 2022 •

edited

Loading

Hotell Jun 7, 2022 •

edited

Loading